-
Notifications
You must be signed in to change notification settings - Fork 13.7k
Stabilize 28 RISC-V target features (riscv_ratified_v2
)
#145948
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Some changes occurred in compiler/rustc_codegen_ssa |
I expect that this PR lands on the version 1.92 cycle (I chose extensions with relatively low risk factors) but happy if it could be merged in the version 1.91 cycle. |
LGTM, these are all officially ratified RISC-V extensions. @rfcbot merge |
Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:
No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns. |
Consideration: Feature Naming (for
|
The `Zb` extension does not exist and we instead have the `B` extension which is a superset of the three subextensions: `Zba`, `Zbb` and `Zbs`. This commit fixes this issue and updates the reference URL to the source code of the latest ratified ISA Manual (version 20250508). To align with rust-lang/rust#145948, this commit performs rename, not removal.
Because unstable "B" (incorrectly named as `zb`) was there, this commit adds 28 minus one ("B") new extensions to be stabilized. This commit directly corresponds to rust-lang/rust#145948. References: 1. RISC-V Instruction Set Manual (version 20250508): <https://github.com/riscv/riscv-isa-manual/tree/20250508> 2. RISC-V Profiles (version 1.0 - RVA23 is not stabilized at the time): <https://github.com/riscv/riscv-profiles/tree/v1.0> 3. RISC-V Profiles (RVA23/RVB23-ratified version): <https://github.com/riscv/riscv-profiles/tree/rva23-rvb23-ratified>
The `Zb` extension does not exist and we instead have the `B` extension which is a superset of the three subextensions: `Zba`, `Zbb` and `Zbs`. This commit fixes this issue and updates the reference URL to the source code of the latest ratified ISA Manual (version 20250508). To align with rust-lang/rust#145948, this commit performs rename, not removal.
Because unstable "B" (incorrectly named as `zb`) was there, this commit adds 28 minus one ("B") new extensions to be stabilized. This commit directly corresponds to rust-lang/rust#145948. References: 1. RISC-V Instruction Set Manual (version 20250508): <https://github.com/riscv/riscv-isa-manual/tree/20250508> 2. RISC-V Profiles (version 1.0 - RVA23 is not stabilized at the time): <https://github.com/riscv/riscv-profiles/tree/v1.0> 3. RISC-V Profiles (RVA23/RVB23-ratified version): <https://github.com/riscv/riscv-profiles/tree/rva23-rvb23-ratified>
The `Zb` extension does not exist and we instead have the `B` extension which is a superset of the three subextensions: `Zba`, `Zbb` and `Zbs`. This commit fixes this issue and updates the reference URL to the source code of the latest ratified ISA Manual (version 20250508). To align with rust-lang/rust#145948, this commit performs rename, not removal.
Because unstable "B" (incorrectly named as `zb`) was there, this commit adds 28 minus one ("B") new extensions to be stabilized. This commit directly corresponds to rust-lang/rust#145948. References: 1. RISC-V Instruction Set Manual (version 20250508): <https://github.com/riscv/riscv-isa-manual/tree/20250508> 2. RISC-V Profiles (version 1.0 - RVA23 is not stabilized at the time): <https://github.com/riscv/riscv-profiles/tree/v1.0> 3. RISC-V Profiles (RVA23/RVB23-ratified version): <https://github.com/riscv/riscv-profiles/tree/rva23-rvb23-ratified>
@traviscross Thanks for pointing out! I opened the PR rust-lang/reference#1987 to align with this. |
Because unstable "B" (incorrectly named as `zb`) was there, this commit adds 28 minus one ("B") new extensions to be stabilized. This commit directly corresponds to rust-lang/rust#145948. References: 1. RISC-V Instruction Set Manual (version 20250508): <https://github.com/riscv/riscv-isa-manual/tree/20250508> 2. RISC-V Profiles (version 1.0 - RVA23 is not stabilized at the time): <https://github.com/riscv/riscv-profiles/tree/v1.0> 3. RISC-V Profiles (RVA23/RVB23-ratified version): <https://github.com/riscv/riscv-profiles/tree/rva23-rvb23-ratified>
I can't parse this comment, too many negations.^^ Integer vector registers can also cause ABI issues, if they are sometimes used by the calling convention but not always available. |
Double negation in the first sentence is intended. Still, there are too much negations after that, I have to admit. I'll try if I can improve this.
Did I miss something? In the standard ABI and normal cases, all vector registers are function-local temporaries. |
Please avoid that, it makes things very hard to read and very easy to misunderstand. "No ABI issues will not occur" means that ABI issues will occur which I hope is not what you meant.
I don't know what the "variant calling convention" is. I also don't know the specifics of any of these extensions, or really much about RISC-V at all. I am just replying to a comment that seemed to somehow say that "float registers could cause a problem but integers cannot", which doesn't make sense to me. Any extensions that makes more registers available could become a problem if arguments start to be passed in those registers. If there are no new registers (nor other new machine state) or the registers are never used for arguments, then great, we have no problem. The comment seems to say that something with integer vectors should be stabilized "another time", so I hope we are fine. But I am not sure since I have a hard time parsing that sentence. It first talks about floats and then about ints, but ints happen another time -- so are floats affected now in this PR or not? Sorry for being so nitpicky here, but I'd rather not risk introducing some soundness issue over a misunderstanding caused by a language barrier. Better to double-check. :) |
Understood. I think I need to rewrite the entire paragraph.
The reason behind "floating point registers cause an ABI issue" is something like However, passing something through vector registers require the callee to enable variant calling conventions (e.g. vector intrinsics in future |
b4455e4
to
a410473
Compare
This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
Rebased and made minor adjustments to the commit message (no changes in the code). @RalfJung I think this is at least better than before.
EDIT: Minor correction is made after initial correction. |
I completely agree that. We'd better to discuss it. |
This commit stabilizes RISC-V target features with following constraints: * Describes a ratified extension. * Implemented on Rust 1.88.0 or before. Waiting for three+ version cycles seems sufficient. * Does not disrupt current rustc's target feature + ABI handling. It excludes "E" and all floating point-arithmetic extensions. "Zfinx" family does not involve floating point registers but not stabilizing for now to avoid possible confusion between the "F" extension family. * Not vector-related (floating point and integer). While integer vector subsets should not cause any ABI issues (as they don't use ABI-dependent floating point registers), we need to discuss before stabilizing them. * Supported by the lowest LLVM version supported by rustc. It excludes the "Zacas" extension, newly supported in LLVM 20 (while the minimum LLVM version supported by rustc is 19). List of target features to be stabilized: 1. "b" 2. "za64rs" (no-RT) 3. "za128rs" (no-RT) 4. "zaamo" 5. "zabha" 6. "zalrsc" 7. "zama16b" (no-RT) 8. "zawrs" 9. "zca" 10. "zcb" 11. "zcmop" 12. "zic64b" (no-RT) 13. "zicbom" 14. "zicbop" (no-RT) 15. "zicboz" 16. "ziccamoa" (no-RT) 17. "ziccif" (no-RT) 18. "zicclsm" (no-RT) 19. "ziccrse" (no-RT) 20. "zicntr" 21. "zicond" 22. "zicsr" 23. "zifencei" 24. "zihintntl" 25. "zihintpause" 26. "zihpm" 27. "zimop" 28. "ztso" Of which, 19 of them (28 minus 9 "no-RT" target features) support runtime detection through `std::arch::is_riscv_feature_detected!()`.
a410473
to
873a0fc
Compare
Thanks! I'm still confused about what's happening with integer vectors, but since they are not being stabilized here we don't have to resolve that now. |
This commit stabilizes RISC-V target features with following constraints:
Waiting for three+ version cycles seems sufficient.
target_feature
compat #140570) + ABI (cf. Some-Ctarget-feature
s must be restrained on RISCV #132618) handling.It excludes
E
and all floating point-arithmetic extensions. TheZfinx
family does not involve floating point registers but not stabilizing for now to avoid possible confusion between theF
extension family.While integer vector subsets should not cause any ABI issues (as they don't use ABI-dependent floating point registers), we need to discuss before stabilizing them.
It excludes the
Zacas
extension, newly supported in LLVM 20 (while the minimum LLVM version supported by rustc is 19).List of target features to be stabilized:
b
za64rs
(no-RT)za128rs
(no-RT)zaamo
zabha
zalrsc
zama16b
(no-RT)zawrs
zca
zcb
zcmop
zic64b
(no-RT)zicbom
zicbop
(no-RT)zicboz
ziccamoa
(no-RT)ziccif
(no-RT)zicclsm
(no-RT)ziccrse
(no-RT)zicntr
zicond
zicsr
zifencei
zihintntl
zihintpause
zihpm
zimop
ztso
Of which, 19 of them (28 minus 9 "no-RT" target features) support runtime detection through
std::arch::is_riscv_feature_detected!()
.Corresponding PR for the Reference: rust-lang/reference#1987
Description in older revision(s)
The original text is rewritten to avoid confusion as per @RalfJung's request.
This is rewritten to correlate this PR with a public API (instead of an internal crate).
r? @Amanieu
@rustbot label +O-riscv